Skip to content

feat: Recoup Content Agent Slack bot#341

Open
recoup-coding-agent wants to merge 3 commits intotestfrom
feature/content-agent
Open

feat: Recoup Content Agent Slack bot#341
recoup-coding-agent wants to merge 3 commits intotestfrom
feature/content-agent

Conversation

@recoup-coding-agent
Copy link
Copy Markdown
Collaborator

@recoup-coding-agent recoup-coding-agent commented Mar 24, 2026

Summary

  • Scaffolds the Recoup Content Agent following the existing coding-agent pattern
  • Slack @mention handler: @RecoupContentAgent <artist_account_id> [template] [batch=N] [lipsync]
  • Triggers the existing create-content pipeline, polls for results via Trigger.dev, and posts video URLs back to the Slack thread
  • Redis-backed thread state via Chat SDK

New Env Vars Required

Variable Description
SLACK_CONTENT_BOT_TOKEN Bot OAuth token (xoxb-...)
SLACK_CONTENT_SIGNING_SECRET Slack app signing secret
CONTENT_AGENT_CALLBACK_SECRET Shared secret for callback auth

Test plan

  • Set env vars in Vercel
  • Create Slack App with app_mentions:read and chat:write scopes
  • Point event subscription URL to POST /api/content-agent/slack
  • @mention the bot with a valid artist account ID in Slack
  • Verify immediate ack message and video reply in thread

Summary by CodeRabbit

Release Notes

  • New Features

    • Added content agent Slack bot supporting artist mentions to generate marketing content with template and batch options.
    • Introduced campaign generation API endpoint for streaming music release marketing content (press releases, social pitches, social media captions, fan newsletters, curator emails).
    • Added content agent webhook endpoints and callback handlers for platform-specific content processing with status tracking.
    • Added content generation result polling and state management for async content workflows.
  • Documentation

    • Enhanced JSDoc parameter documentation across API routes, content handlers, utilities, and helper functions.

Recoup Agent and others added 3 commits March 24, 2026 02:23
Streams a complete AI-generated music release campaign given artist and
song details. Returns 6 sections in real-time (press release, Spotify
pitch, Instagram captions, TikTok hooks, fan newsletter, curator email)
using XML-style section markers parsed by the frontend.

- lib/launch/validateLaunchBody.ts — Zod validation for launch body
- lib/launch/buildCampaignPrompt.ts — system + user prompt builders
- lib/launch/generateCampaignHandler.ts — streamText handler with auth
- lib/launch/__tests__/validateLaunchBody.test.ts — 8 passing tests
- app/api/launch/route.ts — POST /api/launch route

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add content-agent following the existing coding-agent pattern:
- Slack @mention handler that parses artist_account_id, template, batch, lipsync args
- Triggers existing create-content pipeline via Trigger.dev
- poll-content-run task monitors runs and posts video results back to Slack thread
- Callback endpoint receives poll results and posts to Slack
- Redis-backed thread state via Chat SDK

New env vars required: SLACK_CONTENT_BOT_TOKEN, SLACK_CONTENT_SIGNING_SECRET, CONTENT_AGENT_CALLBACK_SECRET

Co-Authored-By: Paperclip <noreply@paperclip.ing>
@vercel
Copy link
Copy Markdown
Contributor

vercel bot commented Mar 24, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
recoup-api Error Error Mar 24, 2026 4:13pm

Request Review

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 24, 2026

📝 Walkthrough

Walkthrough

This PR introduces comprehensive JSDoc documentation across ~90+ files and adds new content agent and campaign launch features, including dynamic webhook endpoints, Slack integration handlers, thread state management, and AI-powered campaign generation workflows.

Changes

Cohort / File(s) Summary
JSDoc Parameter Documentation
app/api/accounts/[id]/route.ts, app/api/admins/coding/slack/route.ts, app/api/admins/privy/route.ts, app/api/coding-agent/[platform]/route.ts, app/api/songs/analyze/presets/route.ts, app/api/transcribe/route.ts
Added @param JSDoc annotations to route handler functions documenting request and params parameters; no runtime changes.
JSDoc Library Utility Documentation
lib/admins/pr/getPrStatusHandler.ts, lib/admins/privy/*.ts, lib/admins/slack/*.ts, lib/ai/getModel.ts, lib/ai/isEmbedModel.ts
Systematic JSDoc parameter documentation added across admin, AI model, and Slack utilities; no functional changes.
Catalog & Content JSDoc Updates
lib/catalog/*.ts, lib/content/*.ts, lib/credits/getCreditUsage.ts
Added @param tags documenting parameters for catalog queries, content handlers, and credit processing; pure documentation updates.
Additional Utility JSDoc
lib/chat/toolChains/getPrepareStepResult.ts, lib/chats/processCompactChatRequest.ts, lib/coding-agent/*.ts, lib/composio/getCallbackUrl.ts, lib/emails/processAndSendEmail.ts, lib/flamingo/getFlamingoPresetsHandler.ts, lib/github/*.ts, lib/mcp/resolveAccountId.ts, lib/mcp/tools/transcribe/registerTranscribeAudioTool.ts, lib/prompts/getSystemPrompt.ts, lib/slack/*.ts, lib/spotify/getSpotifyFollowers.ts, lib/supabase/**/*.ts, lib/transcribe/*.ts, lib/trigger/triggerCreateContent.ts
Extensive JSDoc parameter annotations across helpers for chat, coding agent, file management, GitHub, MCP, Slack, and database utilities; documentation-only changes.
Content Agent Feature
lib/content-agent/bot.ts, lib/content-agent/types.ts, lib/content-agent/getThread.ts, lib/content-agent/validateEnv.ts, lib/content-agent/validateContentAgentCallback.ts, lib/content-agent/handleContentAgentCallback.ts, lib/content-agent/handlers/handleContentAgentCallback.ts, lib/content-agent/handlers/handleContentAgentMention.ts, lib/content-agent/handlers/registerHandlers.ts
New Slack-backed Chat bot infrastructure with Redis state store, environment validation, callback handling with status branching (running/completed/failed/timeout), mention-triggered content generation workflow with artist resolution and trigger polling, and handler registration system.
Launch/Campaign Feature
lib/launch/buildCampaignPrompt.ts, lib/launch/generateCampaignHandler.ts, lib/launch/validateLaunchBody.ts, app/api/launch/route.ts
New campaign generation system with request validation, system/user prompt building, AI streaming via DEFAULT_MODEL, CORS support, and dynamic/no-cache routing configuration.
Content Agent API Routes
app/api/content-agent/[platform]/route.ts, app/api/content-agent/callback/route.ts
New dynamic webhook endpoints resolving platform-specific handlers from contentAgentBot, with URL verification for Slack and delegation to handleContentAgentCallback; includes waitUntil wrapper for async operations.
Additional Utilities
lib/trigger/triggerPollContentRun.ts
New task trigger for polling content run completion with callback thread ID; integrates with trigger.dev task system.

Sequence Diagram(s)

sequenceDiagram
    participant Client as External Callback
    participant API as /api/content-agent/callback
    participant Handler as handleContentAgentCallback
    participant Validator as validateContentAgentCallback
    participant ThreadMgr as getThread + setState
    participant Slack as Slack API

    Client->>API: POST (with auth header & JSON payload)
    API->>Handler: Pass request
    Handler->>Handler: Extract x-callback-secret header
    Handler->>Handler: Verify against CONTENT_AGENT_CALLBACK_SECRET
    alt Auth Invalid
        Handler-->>Client: 401 Unauthorized (CORS)
    else Auth Valid
        Handler->>Handler: Parse request body as JSON
        alt Parse Error
            Handler-->>Client: 400 Invalid JSON (CORS)
        else Parse Success
            Handler->>Validator: validateContentAgentCallback(parsed)
            alt Validation Error
                Validator-->>Handler: NextResponse (error)
                Handler-->>Client: Return error response
            else Validation Success
                Validator-->>Handler: ContentAgentCallbackBody
                Handler->>ThreadMgr: getThread(validated.threadId)
                ThreadMgr-->>Handler: Thread instance
                Handler->>Handler: Check validated.status
                alt Status: "completed"
                    Handler->>ThreadMgr: setState({status: "completed"})
                    Handler->>Slack: Post formatted video results
                    Handler->>Slack: Post completion summary
                else Status: "failed"
                    Handler->>ThreadMgr: setState({status: "failed"})
                    Handler->>Slack: Post failure message
                else Status: "timeout"
                    Handler->>ThreadMgr: setState({status: "timeout"})
                    Handler->>Slack: Post timeout notice
                end
                Handler-->>Client: 200 {status: "ok"} (CORS)
            end
        end
    end
Loading
sequenceDiagram
    participant User as Slack User
    participant Bot as Content Agent Bot
    participant Handler as handleContentAgentMention
    participant Parser as parseMentionArgs
    participant ArtistSvc as resolveArtistSlug / getArtistContentReadiness
    participant Trigger as triggerCreateContent
    participant Poller as triggerPollContentRun
    participant Thread as Thread State

    User->>Bot: `@content-agent` <artist_id> [template] [batch=N] [lipsync]
    Bot->>Handler: onNewMention event
    Handler->>Parser: parseMentionArgs(text)
    Parser-->>Handler: {artistAccountId, template, batch, lipsync}
    alt Missing artistAccountId
        Handler->>Bot: Post usage guidance
    else artistAccountId Present
        Handler->>ArtistSvc: resolveArtistSlug(artistAccountId)
        alt Artist Not Found
            Handler->>Bot: Post "artist not found" message
        else Artist Resolved
            Handler->>ArtistSvc: Determine GitHub repo
            Handler->>Bot: Post acknowledgment with artist/batch/lipsync/template
            Handler->>Trigger: triggerCreateContent(payload) × batch
            Trigger-->>Handler: Promise.allSettled results
            alt No Successful Runs
                Handler->>Bot: Post failure message
            else Successful Runs
                Handler->>Thread: setState({status: "running", artistAccountId, template, lipsync, batch, runIds})
                Handler->>Poller: triggerPollContentRun({runIds, callbackThreadId})
                Handler->>Bot: Async result reporting initiated
            end
        end
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

The PR combines ~90+ homogeneous JSDoc-only changes with several substantial new modules introducing business logic. The repetitive documentation updates lower individual friction, but the heterogeneous mix—spanning new feature architecture (content agent bot, handlers, state management), validation schemas, API routes, and AI integration (launch campaign)—requires contextual reasoning for each functional section. Logic density in the content agent callback handler (branching on status with Slack posting) and mention handler (artist resolution, trigger polling orchestration) adds complexity.

Possibly related PRs

Suggested reviewers

  • sweetmantech

🚀 Docs flow like prose, handlers bloom with care,
Content agents dancing through Slack's air,
Campaigns stream forth with AI's bright spark,
From seeds of parameters to architectured mark! ✨📝

🚥 Pre-merge checks | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Solid & Clean Code ⚠️ Warning Pull request violates SRP (two functions in buildCampaignPrompt.ts), lacks error handling in generateCampaignHandler.ts, exceeds complexity limits in handleContentAgentMention.ts (143 vs 53-75 lines), and contains unvalidated user inputs enabling prompt injection attacks. Split buildCampaignPrompt.ts per SRP; add try-catch around request.json() and streamText(); refactor handleContentAgentMention by extracting validateMentionInput, resolveGithubRepo, triggerContentBatch; implement sanitizePromptField utility for user inputs.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/content-agent

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 9

Note

Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
app/api/transcribe/route.ts (1)

12-33: ⚠️ Potential issue | 🔴 Critical

Do not accept account_id from request body; derive account from auth context.

This allows account spoofing and bypasses the mandated auth flow. Replace body-provided account_id with validateAuthContext() output, and validate input via Zod before calling processAudioTranscription.

As per coding guidelines: "Never use account_id in request bodies or tool schemas; always derive the account ID from authentication" and "Always use validateAuthContext() for authentication in API routes".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/api/transcribe/route.ts` around lines 12 - 33, Stop reading account_id
from the request body and derive the account ID via validateAuthContext();
remove the body.account_id usage and instead call validateAuthContext() in this
route, extract the authenticated account id and pass that as ownerAccountId to
processAudioTranscription (instead of account_id). Also perform Zod validation
on the incoming body fields (audio_url, artist_account_id, title,
include_timestamps) before calling processAudioTranscription so malformed input
is rejected early; ensure artist_account_id still comes from the validated body
and that the validated include_timestamps is forwarded to includeTimestamps.
🟡 Minor comments (13)
lib/flamingo/getFlamingoPresetsHandler.ts-13-13 (1)

13-13: ⚠️ Potential issue | 🟡 Minor

Complete the JSDoc parameter description.

The @param tag should include a description of what the parameter represents. Currently it's just @param request with no explanation.

📝 Proposed fix to add parameter description
- * `@param` request
+ * `@param` request - The incoming Next.js request object.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/flamingo/getFlamingoPresetsHandler.ts` at line 13, Update the JSDoc for
the getFlamingoPresetsHandler function by replacing the bare "@param request"
tag with a concise description of what the request argument represents (e.g.,
the incoming HTTP request object containing query/body headers used to determine
which Flamingo presets to return), keeping the parameter name "request" so it
matches the function signature; ensure the JSDoc clearly states the expected
shape or purpose of the request (for example that it is the incoming API request
used to read query params or body payload to filter presets).
lib/prompts/getSystemPrompt.ts-16-16 (1)

16-16: ⚠️ Potential issue | 🟡 Minor

Complete the JSDoc parameter description.

The @param params.orgId entry is missing a description. All other parameters in this JSDoc block include descriptions (e.g., "The ID of the room", "The ID of the artist"), but this one only lists the parameter name. This inconsistency reduces documentation quality and maintainability.

📝 Proposed fix to add parameter description
- * `@param` params.orgId
+ * `@param` params.orgId - The ID of the organization (optional)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/prompts/getSystemPrompt.ts` at line 16, The JSDoc for params.orgId is
missing a description; update the JSDoc block in getSystemPrompt.ts (the
getSystemPrompt function's comment) to provide a clear description for
params.orgId (e.g., "The ID of the organization" or similar), matching the style
of the other parameter descriptions so the documentation is consistent and
complete.
lib/coding-agent/handleMergeSuccess.ts-10-11 (1)

10-11: ⚠️ Potential issue | 🟡 Minor

Complete the JSDoc parameter documentation.

The @param tag is missing a description. JSDoc parameters should include both the parameter name and a description of what it represents.

📝 Proposed fix to complete the JSDoc
  *
- * `@param` state
+ * `@param` state - The coding agent thread state containing PR and snapshot information
+ * `@returns` A promise that resolves when cleanup completes
  */
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/coding-agent/handleMergeSuccess.ts` around lines 10 - 11, The JSDoc for
handleMergeSuccess has an incomplete `@param` entry — update the comment above the
handleMergeSuccess function to document the state parameter by name and a short
description of what it represents (e.g., "state - current agent state object
containing merge context, branch info, and any relevant metadata"), ensuring the
`@param` line follows JSDoc format and accurately reflects the shape/purpose of
the state argument used in handleMergeSuccess.
lib/slack/getSlackUserInfo.ts-19-21 (1)

19-21: ⚠️ Potential issue | 🟡 Minor

Complete the JSDoc parameter documentation.

The @param tags are missing descriptions. JSDoc best practice is to include what each parameter represents and its purpose.

📝 Proposed fix to add parameter descriptions
  *
- * `@param` token
- * `@param` userId
+ * `@param` token - Slack Bot OAuth token (xoxb-...)
+ * `@param` userId - Slack user ID to look up
  */
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/slack/getSlackUserInfo.ts` around lines 19 - 21, Add missing JSDoc
descriptions for the parameters of getSlackUserInfo: document what the token
parameter is (e.g., Slack OAuth or bot token used to authenticate API requests)
and what the userId parameter represents (the Slack user ID whose profile/info
will be retrieved), and include expected types and any important format or usage
notes so the `@param` tags are complete and informative.
lib/content/createContentHandler.ts-12-13 (1)

12-13: ⚠️ Potential issue | 🟡 Minor

Complete the JSDoc parameter description.

The @param request tag is missing a description. JSDoc parameters should include a brief explanation of their purpose.

📝 Proposed fix
 /**
  * Handler for POST /api/content/create.
  * Always returns runIds array (KISS — one response shape for single and batch).
  *
- * `@param` request
+ * `@param` request - The incoming HTTP request containing content creation parameters
  */
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/content/createContentHandler.ts` around lines 12 - 13, Update the JSDoc
for the createContentHandler function by adding a brief description to the
existing `@param` request tag explaining what the `request` parameter represents
(e.g., the incoming HTTP request or payload object, its expected shape or
purpose) so the comment clearly documents the input to createContentHandler;
keep the description concise and mention any important fields or types if
applicable.
app/api/coding-agent/[platform]/route.ts-38-40 (1)

38-40: ⚠️ Potential issue | 🟡 Minor

@param params.params should be removed in POST docs too.

Keep only @param params to match the actual signature.

Based on learnings: "Applies to app/api/**/route.ts : All API routes should have JSDoc comments".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/api/coding-agent/`[platform]/route.ts around lines 38 - 40, Remove the
incorrect JSDoc param entry "@param params.params" from the POST route JSDoc and
leave only the correct "@param params" to match the actual handler signature;
update the JSDoc block above the exported POST function (route handler) in
app/api/coding-agent/[platform]/route.ts so it contains a single "@param params"
line describing the route params.
app/api/accounts/[id]/route.ts-26-27 (1)

26-27: ⚠️ Potential issue | 🟡 Minor

Remove redundant @param params.params entry.

params.params is misleading here since params is already documented and destructured in the signature.

Based on learnings: "Applies to app/api/**/route.ts : All API routes should have JSDoc comments".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/api/accounts/`[id]/route.ts around lines 26 - 27, Remove the redundant
JSDoc tag "@param params.params" from the route handler's comment and keep a
single clear "@param params" entry describing the route params (account ID);
update the JSDoc above the exported route handler (the function in
app/api/accounts/[id]/route.ts that destructures params) so it only documents
"params" (or "params.id" in the description) and remove the misleading duplicate
entry.
app/api/coding-agent/[platform]/route.ts-13-15 (1)

13-15: ⚠️ Potential issue | 🟡 Minor

@param params.params should be removed in GET docs.

This param path is not a real function parameter and can confuse generated docs.

Based on learnings: "Applies to app/api/**/route.ts : All API routes should have JSDoc comments".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/api/coding-agent/`[platform]/route.ts around lines 13 - 15, Remove the
incorrect JSDoc entry "@param params.params" from the GET route's documentation;
update the JSDoc for the GET handler (the exported GET function in route.ts) to
only document the real parameter (e.g., "@param params - Route params containing
the platform name") and any other valid params, ensuring there are no nested or
duplicate param lines like "params.params" that don't exist on the function
signature.
app/api/admins/privy/route.ts-18-20 (1)

18-20: ⚠️ Potential issue | 🟡 Minor

Replace empty JSDoc on OPTIONS with a meaningful description.

Line 18–20 currently adds an empty block, which doesn’t document the preflight behavior. Add a short summary (e.g., CORS preflight response and status 204) for maintainability.

As per coding guidelines: "All API routes should have JSDoc comments."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/api/admins/privy/route.ts` around lines 18 - 20, Replace the empty JSDoc
block above the OPTIONS handler with a concise summary that documents this
endpoint as the CORS preflight response for the Privy admin route and states it
returns HTTP 204 No Content; update the JSDoc placed immediately above the
OPTIONS function/handler (the block currently empty above the OPTIONS
declaration) to include a one-line description like "CORS preflight response —
returns 204 No Content" and mention it handles preflight checks for allowed
methods and headers.
lib/github/getRepoGitModules.ts-7-12 (1)

7-12: ⚠️ Potential issue | 🟡 Minor

Remove invalid nested @param tags that contradict the actual function signature.

The function destructures owner, repo, and branch as direct string parameters, but the JSDoc includes incorrect nested entries (@param owner.owner, @param owner.repo, @param owner.branch) that suggest owner is an object. Remove these nested tags to align documentation with the actual signature.

Proposed fix
- * `@param` owner.owner
  * `@param` owner - The GitHub repository owner
  * `@param` repo - The GitHub repository name
  * `@param` branch - The branch to fetch from
- * `@param` owner.repo
- * `@param` owner.branch
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/github/getRepoGitModules.ts` around lines 7 - 12, The JSDoc for
getRepoGitModules contains invalid nested `@param` entries (e.g., `@param`
owner.owner, `@param` owner.repo, `@param` owner.branch) that contradict the
function signature which takes owner, repo, and branch as direct string
parameters; remove those nested tags and ensure only top-level `@param` tags
remain for owner, repo, and branch (and adjust any descriptions if needed) so
the documentation matches the actual getRepoGitModules parameter list.
lib/github/resolveSubmodulePath.ts-9-12 (1)

9-12: ⚠️ Potential issue | 🟡 Minor

Remove invalid nested @param paths from JSDoc.

The function signature destructures parameters directly ({ githubRepo, path }), so @param githubRepo.githubRepo and @param githubRepo.path reference a non-existent parent parameter. These conflict with the correct parameter documentation on lines 10–11 and will confuse JSDoc tools and developers. Remove lines 9 and 12.

Proposed fix
- * `@param` githubRepo.githubRepo
  * `@param` githubRepo - The parent GitHub repository URL
  * `@param` path - The file path to resolve
- * `@param` githubRepo.path
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/github/resolveSubmodulePath.ts` around lines 9 - 12, Remove the invalid
nested JSDoc param entries that reference githubRepo.githubRepo and
githubRepo.path and instead document the actual destructured parameters used by
the function (githubRepo and path); update the JSDoc to contain only two `@param`
tags (e.g., `@param` githubRepo - The parent GitHub repository URL and `@param` path
- The file path to resolve) so the comments match the resolveSubmodulePath
function signature.
lib/github/expandSubmoduleEntries.ts-14-22 (1)

14-22: ⚠️ Potential issue | 🟡 Minor

Fix incorrect JSDoc parameter nesting.

The destructured parameters regularEntries, submoduleEntries, and repo are top-level; they should not be nested under regularEntries.*. Remove the incorrectly scoped @param regularEntries.regularEntries, @param regularEntries.submoduleEntries, and @param regularEntries.repo.* declarations. Add proper @param repo.owner, @param repo.repo, and @param repo.branch documentation instead.

Proposed fix
- * `@param` regularEntries.regularEntries
  * `@param` regularEntries - Non-submodule file tree entries
  * `@param` submoduleEntries - Submodule references (type "commit" from GitHub Trees API)
  * `@param` repo - Repository context for fetching .gitmodules
- * `@param` regularEntries.submoduleEntries
- * `@param` regularEntries.repo
- * `@param` regularEntries.repo.owner
- * `@param` regularEntries.repo.repo
- * `@param` regularEntries.repo.branch
+ * `@param` repo.owner - The GitHub repository owner
+ * `@param` repo.repo - The GitHub repository name
+ * `@param` repo.branch - The branch to fetch from
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/github/expandSubmoduleEntries.ts` around lines 14 - 22, The JSDoc
incorrectly nests parameters under regularEntries; update the comment for the
function (expandSubmoduleEntries) by removing `@param`
regularEntries.regularEntries, `@param` regularEntries.submoduleEntries, and
`@param` regularEntries.repo (and its nested owner/repo/branch lines), and instead
declare top-level params: `@param` regularEntries, `@param` submoduleEntries, and
`@param` repo, plus explicit `@param` repo.owner, `@param` repo.repo, and `@param`
repo.branch entries so the destructured repo fields are documented correctly.
lib/content-agent/getThread.ts-10-11 (1)

10-11: ⚠️ Potential issue | 🟡 Minor

Thread ID parsing may be fragile without validation.

The colon-delimited parsing assumes a specific format (e.g., slack:channelId:threadTs). If threadId is malformed or has an unexpected structure, split(":")[1] could return undefined, resulting in channelId of "adapterName:undefined".

Consider adding defensive validation:

🛡️ Proposed defensive check
 export function getThread(threadId: string) {
+  const parts = threadId.split(":");
+  if (parts.length < 2) {
+    throw new Error(`[content-agent] Invalid threadId format: ${threadId}`);
+  }
-  const adapterName = threadId.split(":")[0];
-  const channelId = `${adapterName}:${threadId.split(":")[1]}`;
+  const adapterName = parts[0];
+  const channelId = `${adapterName}:${parts[1]}`;
   return new ThreadImpl<ContentAgentThreadState>({
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/content-agent/getThread.ts` around lines 10 - 11, The threadId parsing in
getThread.ts that derives adapterName and channelId from threadId.split(":") is
fragile; update the logic that computes adapterName and channelId to first
validate threadId has at least two colon-separated parts (e.g., parts =
threadId.split(":") and parts.length >= 2), handle cases with extra parts (for
formats like "slack:channelId:threadTs") by using parts[0] and parts[1] to build
channelId, and if validation fails return a safe failure (throw a descriptive
error or return null) and/or log the invalid threadId instead of producing
"undefined" in channelId. Ensure references are to the existing adapterName and
channelId variables in getThread.ts so the change is localized.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@app/api/content-agent/`[platform]/route.ts:
- Around line 15-59: Add automated tests for the webhook route to cover the GET
404 path, POST Slack URL-verification short-circuit, and normal dispatch via
contentAgentBot.webhooks: write unit tests that invoke the exported GET and POST
handlers (mocking the params promise to return { platform }), stub
contentAgentBot.webhooks to include a fake handler and assert it is called and
its Response returned, stub contentAgentBot.initialize to avoid startup
side-effects, stub handleUrlVerification to return a Response for the Slack
short-circuit case and assert that initialize and the webhook handler are not
called, and add a test where no handler exists to assert a 404 Response; also
verify the waitUntil callback is accepted by your fake handler to exercise the
after(() => p) path.

In `@app/api/launch/route.ts`:
- Around line 45-51: Add unit/integration tests that cover the new API route's
success and failure paths by exercising POST which calls
generateCampaignHandler: create tests for (1) successful stream response (mock
request body and any downstream services, assert streaming chunks and 200), (2)
auth failures (simulate missing/invalid auth and assert the expected 401/403
response), and (3) invalid JSON/body validation errors (send malformed/invalid
payloads and assert 400 and proper error payload). Mock or stub any external
dependencies used by generateCampaignHandler (auth middleware, validators, and
downstream APIs) so tests are deterministic, and include assertions on status
codes and response body/stream behavior for each case.

In `@lib/content-agent/bot.ts`:
- Around line 21-25: The redis.connect() call in createContentAgentBot() is
being fired-and-forgotten; await the connection so callers get a connected
client (or surface connection errors) before the state adapter is created.
Modify createContentAgentBot() (or add an async initialize() method used by
callers like the GET handler) to await redis.connect() when redis.status ===
"wait", propagate any thrown error instead of catching it in a detached promise,
and only construct the state adapter after the successful await; ensure callers
(including code that currently uses the bot directly) await the
factory/initialize promise so the bot is never used with an unconnected Redis
client.

In `@lib/content-agent/handleContentAgentCallback.ts`:
- Around line 44-61: The branch in handleContentAgentCallback that posts
"Content generation finished but no videos were produced." hides per-run
failures and timeouts; instead, when videos.length === 0 use the
already-computed failed array (and any timeout/other non-completed statuses from
validated.results) to build a detailed message: list each failed run (include
status, error/captionText/videoUrl if present) and the count of
failures/timeouts, and then post that constructed message via thread.post rather
than the generic string so callers can see which runs failed and why (refer to
variables validated.results, videos, failed and the thread.post call to locate
where to make the change).

In `@lib/content-agent/handlers/handleContentAgentMention.ts`:
- Around line 111-140: The code uses Promise.allSettled over
triggerCreateContent but keeps the original batch value when calling
thread.setState and triggerPollContentRun, which misreports runs if some
promises failed; change the logic to inspect the allSettled results, compute
successful runIds (as already done), and if runIds.length === 0 throw or return
an error (already handled), but if some succeeded and some failed either (a)
update the saved batch count to runIds.length before calling thread.setState and
then call triggerPollContentRun with those runIds, or (b) treat any failure as
fatal and bail out so no partially-started batch is recorded; implement the
chosen approach by modifying the block around Promise.allSettled / runIds /
thread.setState / triggerPollContentRun to use the adjusted batch/runIds values
and avoid telling users to retry when some runs are already live.

In `@lib/credits/handleChatCredits.ts`:
- Around line 14-20: The JSDoc for the function in
lib/credits/handleChatCredits.ts incorrectly uses nested param names like
`@param usage.usage`, `@param usage.model`, and `@param usage.accountId`; update
the comment to document the actual top-level parameters from the function
signature (e.g., `@param usage - The language model usage data`, `@param model -
The model ID used for the chat`, `@param accountId - The account ID to deduct
credits from (optional)`), remove the redundant `@param usage` line and any
dot‑notation entries, and ensure the JSDoc parameter names match the
destructured parameters used by the function (look for the function
name/identifier in this file to locate the signature).

In `@lib/launch/buildCampaignPrompt.ts`:
- Around line 8-25: This module currently exports multiple prompt builders
(buildCampaignSystemPrompt and buildCampaignUserPrompt); split them so each file
exports exactly one function: move either buildCampaignSystemPrompt or
buildCampaignUserPrompt into a new file (e.g., create
buildCampaignSystemPrompt.ts or buildCampaignUserPrompt.ts), keep the other in
the original file, update any imports/exports to reference the new filename, and
ensure both functions are exported as default or named per project convention
and all callers (tests/consumers) import the correct symbol name; run
type-checks and tests to confirm no import paths remain broken.
- Around line 26-35: The prompt interpolates raw user fields (artist_name,
song_name, genre, release_date, description) into a marker-sensitive template
and is vulnerable to marker injection; add a small sanitizer function (e.g.
sanitizePromptField) and call it for each symbol before building the string in
buildCampaignPrompt: the sanitizer should strip or normalize newlines to spaces,
remove or escape common section/marker tokens (e.g. lines that begin with
hashes, repeated dashes/equals, or labels like "Artist:", "Song/Release:"), trim
and optionally enforce a max length, then use the sanitized values in the
returned template to prevent downstream parsing breaks.

In `@lib/launch/generateCampaignHandler.ts`:
- Around line 22-35: Wrap the call to request.json() in a try-catch and return a
controlled JSON error response with getCorsHeaders() if parsing fails, and
likewise wrap the streamText(...) invocation (the call that uses DEFAULT_MODEL,
buildCampaignSystemPrompt(), buildCampaignUserPrompt(validated)) in its own
try-catch to catch generation errors and return a consistent NextResponse.json
error with CORS headers; keep the existing validateLaunchBody flow (checking for
NextResponse) and only call streamText after validated succeeds, and ensure any
caught error messages are included in the JSON error body so handlers like
getAccountHandler/createCatalogSongsHandler style are followed.

---

Outside diff comments:
In `@app/api/transcribe/route.ts`:
- Around line 12-33: Stop reading account_id from the request body and derive
the account ID via validateAuthContext(); remove the body.account_id usage and
instead call validateAuthContext() in this route, extract the authenticated
account id and pass that as ownerAccountId to processAudioTranscription (instead
of account_id). Also perform Zod validation on the incoming body fields
(audio_url, artist_account_id, title, include_timestamps) before calling
processAudioTranscription so malformed input is rejected early; ensure
artist_account_id still comes from the validated body and that the validated
include_timestamps is forwarded to includeTimestamps.

---

Minor comments:
In `@app/api/accounts/`[id]/route.ts:
- Around line 26-27: Remove the redundant JSDoc tag "@param params.params" from
the route handler's comment and keep a single clear "@param params" entry
describing the route params (account ID); update the JSDoc above the exported
route handler (the function in app/api/accounts/[id]/route.ts that destructures
params) so it only documents "params" (or "params.id" in the description) and
remove the misleading duplicate entry.

In `@app/api/admins/privy/route.ts`:
- Around line 18-20: Replace the empty JSDoc block above the OPTIONS handler
with a concise summary that documents this endpoint as the CORS preflight
response for the Privy admin route and states it returns HTTP 204 No Content;
update the JSDoc placed immediately above the OPTIONS function/handler (the
block currently empty above the OPTIONS declaration) to include a one-line
description like "CORS preflight response — returns 204 No Content" and mention
it handles preflight checks for allowed methods and headers.

In `@app/api/coding-agent/`[platform]/route.ts:
- Around line 38-40: Remove the incorrect JSDoc param entry "@param
params.params" from the POST route JSDoc and leave only the correct "@param
params" to match the actual handler signature; update the JSDoc block above the
exported POST function (route handler) in
app/api/coding-agent/[platform]/route.ts so it contains a single "@param params"
line describing the route params.
- Around line 13-15: Remove the incorrect JSDoc entry "@param params.params"
from the GET route's documentation; update the JSDoc for the GET handler (the
exported GET function in route.ts) to only document the real parameter (e.g.,
"@param params - Route params containing the platform name") and any other valid
params, ensuring there are no nested or duplicate param lines like
"params.params" that don't exist on the function signature.

In `@lib/coding-agent/handleMergeSuccess.ts`:
- Around line 10-11: The JSDoc for handleMergeSuccess has an incomplete `@param`
entry — update the comment above the handleMergeSuccess function to document the
state parameter by name and a short description of what it represents (e.g.,
"state - current agent state object containing merge context, branch info, and
any relevant metadata"), ensuring the `@param` line follows JSDoc format and
accurately reflects the shape/purpose of the state argument used in
handleMergeSuccess.

In `@lib/content-agent/getThread.ts`:
- Around line 10-11: The threadId parsing in getThread.ts that derives
adapterName and channelId from threadId.split(":") is fragile; update the logic
that computes adapterName and channelId to first validate threadId has at least
two colon-separated parts (e.g., parts = threadId.split(":") and parts.length >=
2), handle cases with extra parts (for formats like "slack:channelId:threadTs")
by using parts[0] and parts[1] to build channelId, and if validation fails
return a safe failure (throw a descriptive error or return null) and/or log the
invalid threadId instead of producing "undefined" in channelId. Ensure
references are to the existing adapterName and channelId variables in
getThread.ts so the change is localized.

In `@lib/content/createContentHandler.ts`:
- Around line 12-13: Update the JSDoc for the createContentHandler function by
adding a brief description to the existing `@param` request tag explaining what
the `request` parameter represents (e.g., the incoming HTTP request or payload
object, its expected shape or purpose) so the comment clearly documents the
input to createContentHandler; keep the description concise and mention any
important fields or types if applicable.

In `@lib/flamingo/getFlamingoPresetsHandler.ts`:
- Line 13: Update the JSDoc for the getFlamingoPresetsHandler function by
replacing the bare "@param request" tag with a concise description of what the
request argument represents (e.g., the incoming HTTP request object containing
query/body headers used to determine which Flamingo presets to return), keeping
the parameter name "request" so it matches the function signature; ensure the
JSDoc clearly states the expected shape or purpose of the request (for example
that it is the incoming API request used to read query params or body payload to
filter presets).

In `@lib/github/expandSubmoduleEntries.ts`:
- Around line 14-22: The JSDoc incorrectly nests parameters under
regularEntries; update the comment for the function (expandSubmoduleEntries) by
removing `@param` regularEntries.regularEntries, `@param`
regularEntries.submoduleEntries, and `@param` regularEntries.repo (and its nested
owner/repo/branch lines), and instead declare top-level params: `@param`
regularEntries, `@param` submoduleEntries, and `@param` repo, plus explicit `@param`
repo.owner, `@param` repo.repo, and `@param` repo.branch entries so the destructured
repo fields are documented correctly.

In `@lib/github/getRepoGitModules.ts`:
- Around line 7-12: The JSDoc for getRepoGitModules contains invalid nested
`@param` entries (e.g., `@param` owner.owner, `@param` owner.repo, `@param`
owner.branch) that contradict the function signature which takes owner, repo,
and branch as direct string parameters; remove those nested tags and ensure only
top-level `@param` tags remain for owner, repo, and branch (and adjust any
descriptions if needed) so the documentation matches the actual
getRepoGitModules parameter list.

In `@lib/github/resolveSubmodulePath.ts`:
- Around line 9-12: Remove the invalid nested JSDoc param entries that reference
githubRepo.githubRepo and githubRepo.path and instead document the actual
destructured parameters used by the function (githubRepo and path); update the
JSDoc to contain only two `@param` tags (e.g., `@param` githubRepo - The parent
GitHub repository URL and `@param` path - The file path to resolve) so the
comments match the resolveSubmodulePath function signature.

In `@lib/prompts/getSystemPrompt.ts`:
- Line 16: The JSDoc for params.orgId is missing a description; update the JSDoc
block in getSystemPrompt.ts (the getSystemPrompt function's comment) to provide
a clear description for params.orgId (e.g., "The ID of the organization" or
similar), matching the style of the other parameter descriptions so the
documentation is consistent and complete.

In `@lib/slack/getSlackUserInfo.ts`:
- Around line 19-21: Add missing JSDoc descriptions for the parameters of
getSlackUserInfo: document what the token parameter is (e.g., Slack OAuth or bot
token used to authenticate API requests) and what the userId parameter
represents (the Slack user ID whose profile/info will be retrieved), and include
expected types and any important format or usage notes so the `@param` tags are
complete and informative.

---

Nitpick comments:
In `@app/api/transcribe/route.ts`:
- Around line 5-8: Replace the placeholder JSDoc above the route handler
(exported POST(req) function) with a concise, descriptive comment that
summarizes the route purpose, expected request body shape, response shape, and
possible errors: e.g., state that POST accepts either multipart/form-data with a
"file" field (audio) or JSON with an "audio" base64 string and optional
"language" string, returns 200 with a JSON body { transcription: string } on
success, and returns 400 for invalid input or 500 for server/transcription
errors; include `@param` req, `@returns`, and `@throws/`@errors tags and a one-line
summary so the JSDoc sits immediately above the POST(req) export.

In `@lib/admins/privy/countNewAccounts.ts`:
- Around line 8-10: Update the JSDoc for the countNewAccounts function by adding
concise descriptions to the `@param` tags: describe what the users parameter
represents (e.g., array of User objects or user count input) and what the period
parameter represents (e.g., time window format like '7d' or start/end Date), so
the comment above countNewAccounts clearly documents the expected types and
semantics of users and period.

In `@lib/admins/privy/fetchPrivyLogins.ts`:
- Around line 23-26: The JSDoc for fetchPrivyLogins is non-informative (only
lists "@param period" with no description); update the JSDoc for the function
fetchPrivyLogins to either remove the block entirely or replace it with a short
summary of what the function does and a clear description of the "period"
parameter (expected type/format/units and effects), and document the return
value if applicable (e.g., what shape or errors) so the comment is meaningful.

In `@lib/admins/privy/getCutoffMs.ts`:
- Around line 8-9: The JSDoc for getCutoffMs is missing a description for the
`@param` period tag; update the comment above the getCutoffMs function to add a
concise description for period (what it represents, allowed values or enum
names, and units if applicable, e.g., "period — time window name
('day'|'week'|'month') used to compute cutoff in milliseconds"), so readers know
how to call getCutoffMs and what each period value means.

In `@lib/admins/privy/getLatestVerifiedAt.ts`:
- Around line 7-8: Add a short descriptive text to the JSDoc `@param` user tag for
the getLatestVerifiedAt function: specify what the `user` parameter represents
(e.g., "the user object or record to inspect" or "User model instance whose
verification timestamps will be checked"), include expected shape/type if known
(e.g., contains verifiedAt or verification history), and any important
preconditions the function requires from `user`.

In `@lib/admins/privy/toMs.ts`:
- Around line 4-5: Update the JSDoc for the toMs function so the `@param`
timestamp tag includes a clear description of expected units and format (e.g.,
"Timestamp in seconds or milliseconds; if in seconds, will be converted to
milliseconds"), referencing the toMs function name so maintainers know which
docblock to edit.

In `@lib/admins/slack/extractGithubPrUrls.ts`:
- Around line 28-30: The JSDoc for extractGithubPrUrls is missing short
descriptions for its parameters; update the comment block above the
extractGithubPrUrls function to add brief `@param` entries for text, attachments,
and blocks (e.g., what type/value each represents and how it’s used) so
IDE/tooltips show actionable info; keep each description one short sentence
mentioning expected type/role (e.g., "text: raw Slack message text to scan for
PR URLs"), and ensure the parameter names exactly match the function signature.

In `@lib/admins/slack/fetchThreadPullRequests.ts`:
- Around line 21-24: Update the JSDoc for fetchThreadPullRequests to include
one-line descriptions for each `@param`: describe token (bot token or OAuth token
used for Slack API calls), channel (Slack channel ID where the thread resides),
and threadTs (parent thread timestamp/ts of the thread); place these short
clarifications on the existing `@param` lines in
lib/admins/slack/fetchThreadPullRequests.ts so tooling and future readers
immediately understand each parameter's purpose.

In `@lib/ai/isEmbedModel.ts`:
- Line 7: Update the JSDoc for the isEmbedModel function by adding a short
description to the existing `@param` m tag (e.g., "Gateway model entry to
evaluate") so the parameter is documented; locate the JSDoc above the
isEmbedModel function and replace the bare "@param m" with "@param m - Gateway
model entry to evaluate" (or equivalent concise wording) to make the annotation
actionable for readers and tooling.

In `@lib/catalog/formatCatalogSongsAsCSV.ts`:
- Around line 5-6: Update the JSDoc for the formatCatalogSongsAsCSV function to
include a descriptive `@param` for songs: explain that songs is the array of song
objects (or the expected shape, e.g., objects containing title, artist, album,
duration, etc.) that will be formatted into CSV; ensure the description clearly
states the parameter type and purpose so IDE tooltips and generated docs display
meaningful information.

In `@lib/catalog/getCatalogDataAsCSV.ts`:
- Around line 6-7: The JSDoc for the getCatalogDataAsCSV function has an
incomplete `@param`; update the comment for the `@param` catalogId in the
getCatalogDataAsCSV JSDoc to describe what the parameter represents (e.g.,
"catalogId - the unique identifier of the catalog to export, typically a string
ID or UUID") so callers understand the expected value and purpose.

In `@lib/catalog/getCatalogSongs.ts`:
- Around line 28-34: The JSDoc for getCatalogSongs is too sparse—update the
comment block above the getCatalogSongs function to document each parameter
(catalogId: string — catalog identifier, pageSize: number — max items per page,
include valid range or default, page: number — 0-based or 1-based index and
default, artistName?: string — optional filter) and add an `@returns` describing
the returned Promise shape (e.g., paginated list of songs and metadata or error
conditions). Keep descriptions short, mention any constraints/defaults, and
ensure the param names exactly match the function signature (catalogId,
pageSize, page, artistName) and include `@returns` with the return type summary.

In `@lib/chats/processCompactChatRequest.ts`:
- Around line 20-22: Update the JSDoc for the processCompactChatRequest function
to add clear descriptions for the nested params keys (params.chatId,
params.prompt, params.accountId): state what each field represents, expected
type/format, whether it is required or optional, and any constraints (e.g.,
chatId is a UUID or string identifier, prompt is the user message text,
accountId is the tenant/user account identifier). Edit the comment block above
the processCompactChatRequest function (the `@param` params entry and its nested
`@param` params.* lines) to include these concise descriptions so callers and
maintainers understand intent and format.

In `@lib/content-agent/getThread.ts`:
- Around line 9-16: Duplicate thread reconstruction logic in getThread is
implemented in ContentAgent's getThread (using ThreadImpl and
ContentAgentThreadState) and in CodingAgent's getThread; extract a single
generic utility function getThread<TState>(threadId: string) that parses
adapterName and channelId and returns new ThreadImpl<TState>({ adapterName, id:
threadId, channelId }), place it in a shared module and update both existing
getThread implementations to import and call this generic getThread with their
specific state types (e.g., getThread<ContentAgentThreadState> and
getThread<CodingAgentThreadState>) so ThreadImpl usage and parsing logic are
centralized.

In `@lib/content-agent/handlers/registerHandlers.ts`:
- Around line 9-10: The module currently calls
registerOnNewMention(contentAgentBot) and
registerOnSubscribedMessage(contentAgentBot) at import time which causes side
effects; change this to export an explicit initialization function (e.g.,
initRegisterHandlers or registerContentAgentHandlers) that performs these calls
instead, and add an internal idempotency guard (a module-level boolean like
"registered") so repeated calls are safe; update callers/tests to invoke the new
init function during startup or test setup rather than relying on module import
side-effects, ensuring registerOnNewMention, registerOnSubscribedMessage, and
contentAgentBot are referenced inside the init function only.

In `@lib/content/contentTemplates.ts`:
- Around line 28-31: Update the JSDoc for the function in contentTemplates.ts to
include a short description of the function and a descriptive `@param` for
template that states the expected format/source (e.g., "template: string |
TemplateObject - the HTML/text template or template identifier used to render
content; may include placeholders like {{name}} and should come from the
templates directory or template service"), so developers know what type/value
and origin are expected; update any additional tags (returns, throws) if
applicable to match the implementation.

In `@lib/content/createContentHandler.ts`:
- Around line 15-87: The createContentHandler function is overlong and mixes
responsibilities; extract the GitHub repo resolution, payload construction, and
batch trigger logic into helpers to reduce size and improve testability:
implement resolveGithubRepo(params: {accountId, artistAccountId, artistSlug})
that uses getArtistContentReadiness and falls back to selectAccountSnapshots
(throwing a clear error if none), implement buildContentPayload(validated,
githubRepo) to create the payload object, and implement
executeBatchTriggers(payload, count) that calls triggerCreateContent via
Promise.allSettled and returns { runIds, failedCount }; then refactor
createContentHandler to call these helpers and keep only orchestration, error
handling, and response formatting (using the same response shape).

In `@lib/content/getArtistContentReadiness.ts`:
- Around line 25-29: The JSDoc uses an auto-generated parameter name `root0`
which is unclear; update the docblock for the function getArtistContentReadiness
to use a meaningful parameter name such as `options` (or `params`) and rename
all related JSDoc tags from `@param root0.accountId`, `@param
root0.artistAccountId`, `@param root0.artistSlug` to `@param options.accountId`,
`@param options.artistAccountId`, `@param options.artistSlug` so the docs
clearly match the function signature and intent.

In `@lib/content/validateCreateContentBody.ts`:
- Around line 43-44: Update the JSDoc for the validateCreateContentBody
function: add a descriptive `@param` for the request argument (include expected
type/shape, e.g., Request or a specific DTO and what fields are required) and
add a `@returns` tag describing the return type and possible values (e.g., boolean
or an object with validation errors). Ensure the JSDoc sits immediately above
the validateCreateContentBody declaration so IDEs pick up the types and
descriptions.

In `@lib/mcp/resolveAccountId.ts`:
- Around line 19-20: Update the JSDoc for resolveAccountId to provide clear
descriptions for the params object properties: document what params.authInfo
contains (e.g., authentication context/token, the fields used such as userId,
tenantId, or permissions) and how it is used by resolveAccountId, and describe
params.accountIdOverride (e.g., an optional account ID that, when present, takes
precedence over values derived from authInfo or defaults). Mention expected
types and whether the fields are optional/required and any validation/format
expectations so future readers understand purpose and constraints.

In `@lib/mcp/tools/transcribe/registerTranscribeAudioTool.ts`:
- Around line 18-21: Add a meaningful JSDoc for the registerTranscribeAudioTool
function: describe what the function does (registers the "transcribe_audio" tool
on the provided server), document the server parameter (type and role, e.g.,
Express or Hapi server instance used to attach routes/handlers), mention side
effects (mutates server by registering routes/tools), and include a `@returns` tag
(void or Promise<void> as appropriate) and any thrown errors; reference
registerTranscribeAudioTool, its server parameter, and the registered
"transcribe_audio" tool in the comment.

In `@lib/slack/getBotChannels.ts`:
- Line 13: Update the JSDoc for the token parameter in getBotChannels to a
concise, descriptive phrase (for example "Slack bot OAuth token") so generated
docs are clear; edit the `@param` token line in the JSDoc block above the
getBotChannels function to replace the vague token comment with the short
description mentioning it's the Slack bot OAuth token used for API calls.

In `@lib/slack/getBotUserId.ts`:
- Around line 11-12: Update the JSDoc for the token parameter in getBotUserId
(in lib/slack/getBotUserId.ts) to clearly state it is the Slack bot OAuth token
used for the auth.test call (e.g., "Slack bot OAuth token used for auth.test to
retrieve the calling bot's user ID"), so future callers know exactly what to
pass; keep the description simple and unambiguous and ensure the `@param` name
matches the function parameter name.

In `@lib/slack/getSlackUserInfo.ts`:
- Around line 23-25: The parameter name userId in getSlackUserInfo conflicts
with our repo-wide "account" terminology but is acceptable at this external-API
boundary; keep the parameter as userId to match Slack's users.info API and add a
short comment above getSlackUserInfo explaining this naming decision (e.g.,
"uses Slack 'user' terminology to match Slack API; internal code should map to
'account' after this boundary") so future readers understand the trade-off and
where mapping to 'account' should occur.

In `@lib/supabase/account_artist_ids/getAccountArtistIds.ts`:
- Around line 11-13: Update the JSDoc for the getAccountArtistIds function: add
clear descriptions for the top-level params object and its nested properties by
documenting params.artistIds and params.accountIds (e.g., "artistIds - optional
array of artist ID strings to filter results" and "accountIds - optional array
of account ID strings to filter results"), indicate whether each is optional,
the expected type, and how they affect function behavior (used as filters), and
keep the param names exactly as in the signature (params, params.artistIds,
params.accountIds) so IDEs and docs pick up the details.
- Around line 44-47: The getAccountArtistIds function currently logs errors and
returns an empty array which hides failures; change its error handling so it
doesn't swallow errors: either throw the original error (preferred for library
functions) from getAccountArtistIds after logging the detailed error, or modify
its return type to a result object (e.g., { data: string[] | null, error: Error
| null }) and return the error instead of an empty array; ensure you update any
callers of getAccountArtistIds to handle the thrown error or the new result
object accordingly and keep the processLogger/console message including the full
error details.

In `@lib/supabase/song_artists/insertSongArtists.ts`:
- Around line 8-10: Add a one-line description to the existing JSDoc `@param` tag
for songArtists in insertSongArtists (the function that upserts song-artist
relationships) explaining the expected shape and intent, e.g., "Array of
song-artist join rows to upsert (each object should contain song_id and
artist_id and optional metadata)"; update the JSDoc above the insertSongArtists
function so it follows the project's documented JSDoc pattern for
lib/supabase/**/*.ts files.

In `@lib/supabase/storage/uploadFileByKey.ts`:
- Around line 7-11: Update the JSDoc for the uploadFileByKey function to add
brief descriptions for each parameter: describe "key" as the storage object
path, "file" as the Blob/File or stream to upload, and "options" as an optional
object; explicitly document options.contentType (default: inferred from the file
or fallback to "application/octet-stream") and options.upsert (default: false,
meaning fail if object exists). Also ensure the module follows the Supabase
pattern by importing the Supabase client from "@/lib/supabase/serverClient" and
that uploadFileByKey's JSDoc includes return type and notes about thrown errors
so callers see expected return and error behavior.

In `@lib/transcribe/processAudioTranscription.ts`:
- Around line 69-72: Update the sparse JSDoc for the function handling audio
transcription (processAudioTranscription) to document expected contentType
values (e.g., "audio/mpeg", "audio/mp3", "audio/wav", "audio/x-m4a" or common
file extension equivalents) and specify fallback behavior: when contentType is
missing or unrecognized the function will treat the input as "mp3". Add a short
summary line describing the function purpose and extend the `@param` contentType
description to list accepted MIME types/variants and explicitly state the
default/fallback of "mp3".
- Around line 10-12: The JSDoc for the params argument in
processAudioTranscription is a placeholder; update the docblock for the
processAudioTranscription function to either remove the empty "@param params"
tag or replace it with a one-line description of the expected keys (e.g.,
audioBuffer/audioPath, language, model, options callback fields) and their
types/optional status so callers know what to pass; reference the params
parameter in the processAudioTranscription function signature and include
concise descriptions for each expected key.

In `@lib/transcribe/saveAudioToFiles.ts`:
- Around line 5-8: The JSDoc above the saveAudioToFiles function is empty;
either flesh it out or remove it. Locate the saveAudioToFiles function in
saveAudioToFiles.ts and either add a proper JSDoc describing the function, each
param (types/purpose) and return value, or delete the empty /** ... */ block so
TypeScript types are relied upon instead.

In `@lib/transcribe/saveTranscriptToFiles.ts`:
- Around line 5-8: The JSDoc above the function is empty and should be completed
or removed; update the JSDoc for the saveTranscriptToFiles function to include a
brief one-line description and a clear `@param` description for the params object
(and document key properties if any), or simply delete the entire /** ... */
block if the TypeScript signature is sufficient, ensuring you reference the same
params identifier used by saveTranscriptToFiles.

In `@lib/transcribe/types.ts`:
- Around line 59-60: Update the JSDoc block in lib/transcribe/types.ts to add a
clear description for the `@param` error tag: state what shape or types of errors
are expected (e.g., Error, string, null/undefined, or a custom
TranscriptionError), whether it can be optional, and how callers should
interpret its fields (message, code, or nested details). Locate the JSDoc that
contains "@param error" and expand the tag to a short sentence describing the
expected error types and any important properties or optionality so the
transcribe-related types and functions can be properly documented.

In `@lib/trigger/triggerPollContentRun.ts`:
- Around line 3-6: The type PollContentRunPayload is declared but not exported,
which prevents reuse across the content agent flow; export PollContentRunPayload
from triggerPollContentRun.ts so other modules (e.g., the poll-content-run task
definition or consumers) can import it for type-safe integration, ensuring the
declaration (type PollContentRunPayload = { runIds: string[]; callbackThreadId:
string; }) is exported.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 33ced150-3053-495f-b3dd-17ac6da85465

📥 Commits

Reviewing files that changed from the base of the PR and between f49cda4 and bd01b28.

⛔ Files ignored due to path filters (28)
  • lib/admins/emails/__tests__/validateGetAdminEmailsQuery.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/admins/pr/__tests__/getPrMergedStatusHandler.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/admins/slack/__tests__/getSlackTagsHandler.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/admins/slack/__tests__/validateGetSlackTagsQuery.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/artists/__tests__/createArtistPostHandler.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/artists/__tests__/validateCreateArtistBody.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/auth/__tests__/validateAuthContext.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/chat/__tests__/integration/chatEndToEnd.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/coding-agent/__tests__/handleGitHubWebhook.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/coding-agent/__tests__/onMergeTestToMainAction.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/content/__tests__/validateCreateContentBody.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/evals/callChatFunctions.ts is excluded by !**/evals/** and included by lib/**
  • lib/evals/callChatFunctionsWithResult.ts is excluded by !**/evals/** and included by lib/**
  • lib/evals/createToolsCalledScorer.ts is excluded by !**/evals/** and included by lib/**
  • lib/evals/extractTextFromResult.ts is excluded by !**/evals/** and included by lib/**
  • lib/evals/extractTextResultFromSteps.ts is excluded by !**/evals/** and included by lib/**
  • lib/evals/getCatalogSongsCountExpected.ts is excluded by !**/evals/** and included by lib/**
  • lib/evals/getSpotifyFollowersExpected.ts is excluded by !**/evals/** and included by lib/**
  • lib/evals/scorers/CatalogAvailability.ts is excluded by !**/evals/** and included by lib/**
  • lib/evals/scorers/QuestionAnswered.ts is excluded by !**/evals/** and included by lib/**
  • lib/evals/scorers/ToolsCalled.ts is excluded by !**/evals/** and included by lib/**
  • lib/flamingo/__tests__/getFlamingoPresetsHandler.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/launch/__tests__/validateLaunchBody.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/notifications/__tests__/createNotificationHandler.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/notifications/__tests__/validateCreateNotificationBody.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/tasks/__tests__/enrichTaskWithTriggerInfo.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/tasks/__tests__/getTaskRunHandler.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/tasks/__tests__/validateGetTaskRunQuery.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
📒 Files selected for processing (78)
  • app/api/accounts/[id]/route.ts
  • app/api/admins/coding/slack/route.ts
  • app/api/admins/privy/route.ts
  • app/api/coding-agent/[platform]/route.ts
  • app/api/content-agent/[platform]/route.ts
  • app/api/content-agent/callback/route.ts
  • app/api/launch/route.ts
  • app/api/songs/analyze/presets/route.ts
  • app/api/transcribe/route.ts
  • lib/admins/pr/getPrStatusHandler.ts
  • lib/admins/privy/countNewAccounts.ts
  • lib/admins/privy/fetchPrivyLogins.ts
  • lib/admins/privy/getCutoffMs.ts
  • lib/admins/privy/getLatestVerifiedAt.ts
  • lib/admins/privy/toMs.ts
  • lib/admins/slack/extractGithubPrUrls.ts
  • lib/admins/slack/fetchThreadPullRequests.ts
  • lib/ai/getModel.ts
  • lib/ai/isEmbedModel.ts
  • lib/catalog/formatCatalogSongsAsCSV.ts
  • lib/catalog/getCatalogDataAsCSV.ts
  • lib/catalog/getCatalogSongs.ts
  • lib/catalog/getCatalogs.ts
  • lib/chat/toolChains/getPrepareStepResult.ts
  • lib/chats/processCompactChatRequest.ts
  • lib/coding-agent/encodeGitHubThreadId.ts
  • lib/coding-agent/handleMergeSuccess.ts
  • lib/coding-agent/parseMergeActionId.ts
  • lib/coding-agent/parseMergeTestToMainActionId.ts
  • lib/composio/getCallbackUrl.ts
  • lib/content-agent/bot.ts
  • lib/content-agent/getThread.ts
  • lib/content-agent/handleContentAgentCallback.ts
  • lib/content-agent/handlers/handleContentAgentCallback.ts
  • lib/content-agent/handlers/handleContentAgentMention.ts
  • lib/content-agent/handlers/registerHandlers.ts
  • lib/content-agent/types.ts
  • lib/content-agent/validateContentAgentCallback.ts
  • lib/content-agent/validateEnv.ts
  • lib/content/contentTemplates.ts
  • lib/content/createContentHandler.ts
  • lib/content/getArtistContentReadiness.ts
  • lib/content/getArtistFileTree.ts
  • lib/content/getArtistRootPrefix.ts
  • lib/content/getContentValidateHandler.ts
  • lib/content/isCompletedRun.ts
  • lib/content/persistCreateContentRunVideo.ts
  • lib/content/validateCreateContentBody.ts
  • lib/content/validateGetContentEstimateQuery.ts
  • lib/content/validateGetContentValidateQuery.ts
  • lib/credits/getCreditUsage.ts
  • lib/credits/handleChatCredits.ts
  • lib/emails/processAndSendEmail.ts
  • lib/flamingo/getFlamingoPresetsHandler.ts
  • lib/github/expandSubmoduleEntries.ts
  • lib/github/getRepoGitModules.ts
  • lib/github/resolveSubmodulePath.ts
  • lib/launch/buildCampaignPrompt.ts
  • lib/launch/generateCampaignHandler.ts
  • lib/launch/validateLaunchBody.ts
  • lib/mcp/resolveAccountId.ts
  • lib/mcp/tools/transcribe/registerTranscribeAudioTool.ts
  • lib/prompts/getSystemPrompt.ts
  • lib/slack/getBotChannels.ts
  • lib/slack/getBotUserId.ts
  • lib/slack/getSlackUserInfo.ts
  • lib/spotify/getSpotifyFollowers.ts
  • lib/supabase/account_artist_ids/getAccountArtistIds.ts
  • lib/supabase/account_workspace_ids/getAccountWorkspaceIds.ts
  • lib/supabase/files/createFileRecord.ts
  • lib/supabase/song_artists/insertSongArtists.ts
  • lib/supabase/storage/uploadFileByKey.ts
  • lib/transcribe/processAudioTranscription.ts
  • lib/transcribe/saveAudioToFiles.ts
  • lib/transcribe/saveTranscriptToFiles.ts
  • lib/transcribe/types.ts
  • lib/trigger/triggerCreateContent.ts
  • lib/trigger/triggerPollContentRun.ts

Comment on lines +15 to +59
export async function GET(
request: NextRequest,
{ params }: { params: Promise<{ platform: string }> },
) {
const { platform } = await params;

const handler = contentAgentBot.webhooks[platform as keyof typeof contentAgentBot.webhooks];

if (!handler) {
return new Response("Unknown platform", { status: 404 });
}

return handler(request, { waitUntil: p => after(() => p) });
}

/**
* POST /api/content-agent/[platform]
*
* Webhook endpoint for the content agent bot.
* Handles Slack webhooks via dynamic [platform] segment.
*
* @param request - The incoming webhook request
* @param params - Route params containing the platform name
*/
export async function POST(
request: NextRequest,
{ params }: { params: Promise<{ platform: string }> },
) {
const { platform } = await params;

if (platform === "slack") {
const verification = await handleUrlVerification(request);
if (verification) return verification;
}

await contentAgentBot.initialize();

const handler = contentAgentBot.webhooks[platform as keyof typeof contentAgentBot.webhooks];

if (!handler) {
return new Response("Unknown platform", { status: 404 });
}

return handler(request, { waitUntil: p => after(() => p) });
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Please add automated coverage for these webhook branches.

I don't see tests in this PR for the 404 path, Slack URL-verification short-circuit, or normal dispatch through contentAgentBot.webhooks. This is internet-facing ingress, so relying on the manual Slack/Vercel checklist is pretty fragile.

As per coding guidelines "Write tests for new API endpoints covering all success and error paths".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/api/content-agent/`[platform]/route.ts around lines 15 - 59, Add
automated tests for the webhook route to cover the GET 404 path, POST Slack
URL-verification short-circuit, and normal dispatch via
contentAgentBot.webhooks: write unit tests that invoke the exported GET and POST
handlers (mocking the params promise to return { platform }), stub
contentAgentBot.webhooks to include a fake handler and assert it is called and
its Response returned, stub contentAgentBot.initialize to avoid startup
side-effects, stub handleUrlVerification to return a Response for the Slack
short-circuit case and assert that initialize and the webhook handler are not
called, and add a test where no handler exists to assert a 404 Response; also
verify the waitUntil callback is accepted by your fake handler to exercise the
after(() => p) path.

Comment on lines +45 to +51
export async function POST(request: NextRequest): Promise<Response> {
return generateCampaignHandler(request);
}

export const dynamic = "force-dynamic";
export const fetchCache = "force-no-store";
export const revalidate = 0;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Add endpoint tests before merge.

This new API route needs coverage for success and failure paths (auth failures, invalid JSON/body validation errors, and successful stream response).

As per coding guidelines: app/api/**/route.ts: Write tests for new API endpoints covering all success and error paths.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/api/launch/route.ts` around lines 45 - 51, Add unit/integration tests
that cover the new API route's success and failure paths by exercising POST
which calls generateCampaignHandler: create tests for (1) successful stream
response (mock request body and any downstream services, assert streaming chunks
and 200), (2) auth failures (simulate missing/invalid auth and assert the
expected 401/403 response), and (3) invalid JSON/body validation errors (send
malformed/invalid payloads and assert 400 and proper error payload). Mock or
stub any external dependencies used by generateCampaignHandler (auth middleware,
validators, and downstream APIs) so tests are deterministic, and include
assertions on status codes and response body/stream behavior for each case.

Comment on lines +21 to +25
if (redis.status === "wait") {
redis.connect().catch(() => {
throw new Error("[content-agent] Redis failed to connect");
});
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's find and examine the file
find . -type f -name "bot.ts" -path "*/content-agent/*" | head -5

Repository: recoupable/api

Length of output: 84


🏁 Script executed:

# Read the file to examine the redis connection handling
cat -n lib/content-agent/bot.ts

Repository: recoupable/api

Length of output: 1870


🏁 Script executed:

# Check how redis is imported and what its type is to understand connection behavior
cat -n lib/redis/connection.ts | head -30

Repository: recoupable/api

Length of output: 871


🏁 Script executed:

# Check if there are any await calls or error handling for contentAgentBot downstream
rg -n "contentAgentBot" --type ts --type tsx | head -20

Repository: recoupable/api

Length of output: 85


🏁 Script executed:

# Search for contentAgentBot usage (without tsx type which causes error)
rg -n "contentAgentBot" --type ts

Repository: recoupable/api

Length of output: 1084


🏁 Script executed:

# Check if createContentAgentBot is called anywhere else or if singleton is the only usage
rg -n "createContentAgentBot" --type ts

Repository: recoupable/api

Length of output: 331


🏁 Script executed:

# Let's check what Chat.initialize() does and when redis connection is expected
rg -n "initialize" lib/content-agent/bot.ts -A 5

Repository: recoupable/api

Length of output: 40


🏁 Script executed:

# Check if there are any other redis connection calls or error handling patterns in the codebase
rg -n "redis\.connect\(\)" --type ts

Repository: recoupable/api

Length of output: 239


🏁 Script executed:

# Check the other bot files to see if they have the same pattern and how they're used
cat -n lib/slack/chat/bot.ts | sed -n '20,40p'

Repository: recoupable/api

Length of output: 798


🏁 Script executed:

cat -n lib/coding-agent/bot.ts | sed -n '20,40p'

Repository: recoupable/api

Length of output: 893


🏁 Script executed:

# Check the Chat SDK type to understand if initialize() handles redis readiness
rg -n "initialize" lib/slack/chat/bot.ts -B 2 -A 10 | head -30

Repository: recoupable/api

Length of output: 40


🏁 Script executed:

# Look at how the state adapter interacts with redis readiness
rg -n "createIoRedisState" --type ts -B 2 -A 8 | head -40

Repository: recoupable/api

Length of output: 2418


🏁 Script executed:

# Check if there's any global error handler for unhandled promise rejections
rg -n "unhandledRejection" --type ts

Repository: recoupable/api

Length of output: 40


🏁 Script executed:

# Look at how initialize() is actually implemented to see if it waits for redis
rg -n "\.initialize\(\)" lib/content-agent/ -B 2 -A 5

Repository: recoupable/api

Length of output: 40


🏁 Script executed:

# Check the app routes that use contentAgentBot to see the full flow
cat -n app/api/content-agent/callback/route.ts | head -30

Repository: recoupable/api

Length of output: 737


🏁 Script executed:

cat -n app/api/content-agent/\[platform\]/route.ts | head -30

Repository: recoupable/api

Length of output: 1180


🏁 Script executed:

# Let's check the Chat.initialize() method from the test mock to understand what it does
cat -n lib/coding-agent/__tests__/bot.test.ts | sed -n '14,25p'

Repository: recoupable/api

Length of output: 361


🏁 Script executed:

# Search for any documentation about how the state adapter expects redis to be ready
rg -n "ready" lib/slack/chat/bot.ts -B 2 -A 2

Repository: recoupable/api

Length of output: 272


Await Redis connection before creating the state adapter instead of using fire-and-forget error handling.

The redis.connect() call at lines 21–25 is not awaited, which means createContentAgentBot() returns before the Redis client is guaranteed to be connected. If redis.connect() rejects, the error is trapped in a detached promise and never propagates to the caller. The state adapter is then created with a potentially unconnected Redis client. Additionally, not all callers invoke initialize() before using the bot (e.g., the GET handler at app/api/content-agent/[platform]/route.ts uses it directly).

Move the connection check into an async initialization step that is awaited before the bot is used, or ensure redis.connect() is awaited directly in the bot factory function and all callers handle the promise.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/content-agent/bot.ts` around lines 21 - 25, The redis.connect() call in
createContentAgentBot() is being fired-and-forgotten; await the connection so
callers get a connected client (or surface connection errors) before the state
adapter is created. Modify createContentAgentBot() (or add an async initialize()
method used by callers like the GET handler) to await redis.connect() when
redis.status === "wait", propagate any thrown error instead of catching it in a
detached promise, and only construct the state adapter after the successful
await; ensure callers (including code that currently uses the bot directly)
await the factory/initialize promise so the bot is never used with an
unconnected Redis client.

Comment on lines +44 to +61
const results = validated.results ?? [];
const videos = results.filter(r => r.status === "completed" && r.videoUrl);
const failed = results.filter(r => r.status === "failed");

if (videos.length > 0) {
const lines = videos.map((v, i) => {
const label = videos.length > 1 ? `**Video ${i + 1}:** ` : "";
const caption = v.captionText ? `\n> ${v.captionText}` : "";
return `${label}${v.videoUrl}${caption}`;
});

if (failed.length > 0) {
lines.push(`\n_${failed.length} run(s) failed._`);
}

await thread.post(lines.join("\n\n"));
} else {
await thread.post("Content generation finished but no videos were produced.");
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Don't collapse failed batches into a generic "no videos" message.

When videos.length === 0, this branch drops the per-run failure context you've already computed, and timeout results are never surfaced at all. A batch that fully failed can end up looking like a benign no-output case.

💡 Suggested fix
       const results = validated.results ?? [];
       const videos = results.filter(r => r.status === "completed" && r.videoUrl);
       const failed = results.filter(r => r.status === "failed");
+      const timedOut = results.filter(r => r.status === "timeout");

       if (videos.length > 0) {
         const lines = videos.map((v, i) => {
           const label = videos.length > 1 ? `**Video ${i + 1}:** ` : "";
           const caption = v.captionText ? `\n> ${v.captionText}` : "";
           return `${label}${v.videoUrl}${caption}`;
         });

-        if (failed.length > 0) {
-          lines.push(`\n_${failed.length} run(s) failed._`);
+        if (failed.length > 0 || timedOut.length > 0) {
+          const summary = [
+            failed.length > 0 ? `${failed.length} failed` : null,
+            timedOut.length > 0 ? `${timedOut.length} timed out` : null,
+          ]
+            .filter(Boolean)
+            .join(", ");
+          lines.push(`\n_${summary}._`);
         }

         await thread.post(lines.join("\n\n"));
       } else {
-        await thread.post("Content generation finished but no videos were produced.");
+        const summary = [
+          failed.length > 0 ? `${failed.length} failed` : null,
+          timedOut.length > 0 ? `${timedOut.length} timed out` : null,
+        ]
+          .filter(Boolean)
+          .join(", ");
+        await thread.post(
+          `Content generation finished but no videos were produced${summary ? ` (${summary})` : ""}.`,
+        );
       }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const results = validated.results ?? [];
const videos = results.filter(r => r.status === "completed" && r.videoUrl);
const failed = results.filter(r => r.status === "failed");
if (videos.length > 0) {
const lines = videos.map((v, i) => {
const label = videos.length > 1 ? `**Video ${i + 1}:** ` : "";
const caption = v.captionText ? `\n> ${v.captionText}` : "";
return `${label}${v.videoUrl}${caption}`;
});
if (failed.length > 0) {
lines.push(`\n_${failed.length} run(s) failed._`);
}
await thread.post(lines.join("\n\n"));
} else {
await thread.post("Content generation finished but no videos were produced.");
const results = validated.results ?? [];
const videos = results.filter(r => r.status === "completed" && r.videoUrl);
const failed = results.filter(r => r.status === "failed");
const timedOut = results.filter(r => r.status === "timeout");
if (videos.length > 0) {
const lines = videos.map((v, i) => {
const label = videos.length > 1 ? `**Video ${i + 1}:** ` : "";
const caption = v.captionText ? `\n> ${v.captionText}` : "";
return `${label}${v.videoUrl}${caption}`;
});
if (failed.length > 0 || timedOut.length > 0) {
const summary = [
failed.length > 0 ? `${failed.length} failed` : null,
timedOut.length > 0 ? `${timedOut.length} timed out` : null,
]
.filter(Boolean)
.join(", ");
lines.push(`\n_${summary}._`);
}
await thread.post(lines.join("\n\n"));
} else {
const summary = [
failed.length > 0 ? `${failed.length} failed` : null,
timedOut.length > 0 ? `${timedOut.length} timed out` : null,
]
.filter(Boolean)
.join(", ");
await thread.post(
`Content generation finished but no videos were produced${summary ? ` (${summary})` : ""}.`,
);
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/content-agent/handleContentAgentCallback.ts` around lines 44 - 61, The
branch in handleContentAgentCallback that posts "Content generation finished but
no videos were produced." hides per-run failures and timeouts; instead, when
videos.length === 0 use the already-computed failed array (and any timeout/other
non-completed statuses from validated.results) to build a detailed message: list
each failed run (include status, error/captionText/videoUrl if present) and the
count of failures/timeouts, and then post that constructed message via
thread.post rather than the generic string so callers can see which runs failed
and why (refer to variables validated.results, videos, failed and the
thread.post call to locate where to make the change).

Comment on lines +111 to +140
const results = await Promise.allSettled(
Array.from({ length: batch }, () => triggerCreateContent(payload)),
);
const runIds = results
.filter(r => r.status === "fulfilled")
.map(r => (r as PromiseFulfilledResult<{ id: string }>).value.id);

if (runIds.length === 0) {
await thread.post("Failed to trigger content creation. Please try again.");
return;
}

// Set thread state
await thread.setState({
status: "running",
artistAccountId,
template,
lipsync,
batch,
runIds,
});

// Trigger polling task
await triggerPollContentRun({
runIds,
callbackThreadId: thread.id,
});
} catch (error) {
console.error("[content-agent] onNewMention error:", error);
await thread.post("Something went wrong starting content generation. Please try again.");
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Don't report a partially-started batch as if all requested runs are active.

Promise.allSettled() lets this continue with fewer runIds than the requested batch, but the stored state still keeps the original batch size. If triggerPollContentRun() then throws, the thread is told to retry even though some runs may already be live, which can create duplicate content.

💡 Suggested direction
       const runIds = results
         .filter(r => r.status === "fulfilled")
         .map(r => (r as PromiseFulfilledResult<{ id: string }>).value.id);
+      const startedBatch = runIds.length;
+      const failedToStart = batch - startedBatch;

       if (runIds.length === 0) {
         await thread.post("Failed to trigger content creation. Please try again.");
         return;
       }
+
+      if (failedToStart > 0) {
+        await thread.post(
+          `${failedToStart} of ${batch} content run(s) failed to start. I'll keep tracking the ${startedBatch} that did.`,
+        );
+      }

       // Set thread state
       await thread.setState({
         status: "running",
         artistAccountId,
         template,
         lipsync,
-        batch,
+        batch: startedBatch,
         runIds,
       });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/content-agent/handlers/handleContentAgentMention.ts` around lines 111 -
140, The code uses Promise.allSettled over triggerCreateContent but keeps the
original batch value when calling thread.setState and triggerPollContentRun,
which misreports runs if some promises failed; change the logic to inspect the
allSettled results, compute successful runIds (as already done), and if
runIds.length === 0 throw or return an error (already handled), but if some
succeeded and some failed either (a) update the saved batch count to
runIds.length before calling thread.setState and then call triggerPollContentRun
with those runIds, or (b) treat any failure as fatal and bail out so no
partially-started batch is recorded; implement the chosen approach by modifying
the block around Promise.allSettled / runIds / thread.setState /
triggerPollContentRun to use the adjusted batch/runIds values and avoid telling
users to retry when some runs are already live.

Comment on lines +14 to +20
*
* @param usage.usage
* @param usage - The language model usage data
* @param model - The model ID used for the chat
* @param accountId - The account ID to deduct credits from (optional)
* @param usage.model
* @param usage.accountId
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Fix incorrect JSDoc parameter documentation.

The JSDoc uses dot notation (@param usage.usage, @param usage.model, @param usage.accountId) which suggests nested properties, but the function signature shows these are three separate top-level parameters in the destructured object, not nested within usage. Line 16's @param usage is also now redundant given line 15's @param usage.usage.

This will cause documentation generators and IDEs to display incorrect parameter information.

📝 Proposed fix for the JSDoc
  *
- * `@param` usage.usage
- * `@param` usage - The language model usage data
- * `@param` model - The model ID used for the chat
- * `@param` accountId - The account ID to deduct credits from (optional)
- * `@param` usage.model
- * `@param` usage.accountId
+ * `@param` params - The parameters object
+ * `@param` params.usage - The language model usage data
+ * `@param` params.model - The model ID used for the chat
+ * `@param` params.accountId - The account ID to deduct credits from (optional)
  */
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
*
* @param usage.usage
* @param usage - The language model usage data
* @param model - The model ID used for the chat
* @param accountId - The account ID to deduct credits from (optional)
* @param usage.model
* @param usage.accountId
*
* `@param` params - The parameters object
* `@param` params.usage - The language model usage data
* `@param` params.model - The model ID used for the chat
* `@param` params.accountId - The account ID to deduct credits from (optional)
*/
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/credits/handleChatCredits.ts` around lines 14 - 20, The JSDoc for the
function in lib/credits/handleChatCredits.ts incorrectly uses nested param names
like `@param usage.usage`, `@param usage.model`, and `@param usage.accountId`;
update the comment to document the actual top-level parameters from the function
signature (e.g., `@param usage - The language model usage data`, `@param model -
The model ID used for the chat`, `@param accountId - The account ID to deduct
credits from (optional)`), remove the redundant `@param usage` line and any
dot‑notation entries, and ensure the JSDoc parameter names match the
destructured parameters used by the function (look for the function
name/identifier in this file to locate the signature).

Comment on lines +8 to +25
export function buildCampaignSystemPrompt(): string {
return `You are an expert music industry publicist and marketing strategist with 15+ years of experience
launching indie and major label artists. You write compelling, professional, and authentic music PR content
that sounds human — never generic. Your press releases get picked up by music blogs. Your Spotify pitches
get playlisted. Your emails get replies.

Always write as if this artist and song are genuinely exciting. Use vivid, specific language.
Avoid clichés like "sonic journey" or "genre-defying."
Output each section EXACTLY as instructed with the section markers provided — no extra text outside the markers.`;
}

/**
* Builds the user prompt for the release campaign generator.
*
* @param body - Validated launch request body
* @returns The formatted user prompt
*/
export function buildCampaignUserPrompt(body: LaunchBody): string {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Split this module to satisfy SRP export rule.

This file exports multiple functions; move one prompt builder to its own file so each file exposes one exported function.

As per coding guidelines: lib/**/*.ts: Apply Single Responsibility Principle (SRP): one exported function per file; each file should do one thing well.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/launch/buildCampaignPrompt.ts` around lines 8 - 25, This module currently
exports multiple prompt builders (buildCampaignSystemPrompt and
buildCampaignUserPrompt); split them so each file exports exactly one function:
move either buildCampaignSystemPrompt or buildCampaignUserPrompt into a new file
(e.g., create buildCampaignSystemPrompt.ts or buildCampaignUserPrompt.ts), keep
the other in the original file, update any imports/exports to reference the new
filename, and ensure both functions are exported as default or named per project
convention and all callers (tests/consumers) import the correct symbol name; run
type-checks and tests to confirm no import paths remain broken.

Comment on lines +26 to +35
const { artist_name, song_name, genre, release_date, description } = body;

const context = description ? `\nAdditional context: ${description}` : "";

return `Generate a complete music release campaign for:
Artist: ${artist_name}
Song/Release: ${song_name}
Genre: ${genre}
Release Date: ${release_date}${context}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Guard prompt structure from marker injection in user fields.

artist_name, song_name, genre, release_date, and description are inserted raw into a marker-sensitive prompt format. Inputs containing section markers can break downstream parsing of streamed sections.

💡 Suggested hardening
 import type { LaunchBody } from "./validateLaunchBody";
 
+function sanitizePromptField(value: string): string {
+  return value
+    .replaceAll("[SECTION:", "")
+    .replaceAll("[/SECTION:", "")
+    .trim();
+}
+
 /**
  * Builds the system prompt for the release campaign generator.
  *
  * `@returns` The system prompt string
  */
@@
 export function buildCampaignUserPrompt(body: LaunchBody): string {
   const { artist_name, song_name, genre, release_date, description } = body;
 
-  const context = description ? `\nAdditional context: ${description}` : "";
+  const safeArtist = sanitizePromptField(artist_name);
+  const safeSong = sanitizePromptField(song_name);
+  const safeGenre = sanitizePromptField(genre);
+  const safeReleaseDate = sanitizePromptField(release_date);
+  const safeDescription = description ? sanitizePromptField(description) : "";
+  const context = safeDescription ? `\nAdditional context: ${safeDescription}` : "";
 
   return `Generate a complete music release campaign for:
-Artist: ${artist_name}
-Song/Release: ${song_name}
-Genre: ${genre}
-Release Date: ${release_date}${context}
+Artist: ${safeArtist}
+Song/Release: ${safeSong}
+Genre: ${safeGenre}
+Release Date: ${safeReleaseDate}${context}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/launch/buildCampaignPrompt.ts` around lines 26 - 35, The prompt
interpolates raw user fields (artist_name, song_name, genre, release_date,
description) into a marker-sensitive template and is vulnerable to marker
injection; add a small sanitizer function (e.g. sanitizePromptField) and call it
for each symbol before building the string in buildCampaignPrompt: the sanitizer
should strip or normalize newlines to spaces, remove or escape common
section/marker tokens (e.g. lines that begin with hashes, repeated
dashes/equals, or labels like "Artist:", "Song/Release:"), trim and optionally
enforce a max length, then use the sanitized values in the returned template to
prevent downstream parsing breaks.

Comment on lines +22 to +35
const json = await request.json();
const validated = validateLaunchBody(json);
if (validated instanceof NextResponse) {
return validated;
}

const result = streamText({
model: DEFAULT_MODEL,
system: buildCampaignSystemPrompt(),
prompt: buildCampaignUserPrompt(validated),
});

return result.toTextStreamResponse({ headers: getCorsHeaders() });
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

find . -name "generateCampaignHandler.ts" -type f

Repository: recoupable/api

Length of output: 97


🏁 Script executed:

wc -l lib/launch/generateCampaignHandler.ts

Repository: recoupable/api

Length of output: 98


🏁 Script executed:

cat -n lib/launch/generateCampaignHandler.ts

Repository: recoupable/api

Length of output: 1549


🏁 Script executed:

rg -A 5 -B 5 "getCorsHeaders" --type ts

Repository: recoupable/api

Length of output: 50371


🏁 Script executed:

rg -B 3 "request\.json\(\)" --type ts

Repository: recoupable/api

Length of output: 6044


🏁 Script executed:

ast-grep --pattern 'export async function generateCampaignHandler($_) {
  $$$
}'

Repository: recoupable/api

Length of output: 40


Add error handling for JSON parsing and stream generation failures.

request.json() and streamText() can throw unhandled exceptions. Wrap both in try-catch blocks and return controlled JSON errors with CORS headers, consistent with other handlers in the codebase (e.g., getAccountHandler, createCatalogSongsHandler).

✅ Suggested error-handling patch
 export async function generateCampaignHandler(request: NextRequest): Promise<Response> {
   const authResult = await validateAuthContext(request);
   if (authResult instanceof NextResponse) {
     return authResult;
   }
 
-  const json = await request.json();
+  let json: unknown;
+  try {
+    json = await request.json();
+  } catch {
+    return NextResponse.json(
+      { status: "error", error: "Invalid JSON body" },
+      { status: 400, headers: getCorsHeaders() },
+    );
+  }
+
   const validated = validateLaunchBody(json);
   if (validated instanceof NextResponse) {
     return validated;
   }
 
-  const result = streamText({
-    model: DEFAULT_MODEL,
-    system: buildCampaignSystemPrompt(),
-    prompt: buildCampaignUserPrompt(validated),
-  });
-
-  return result.toTextStreamResponse({ headers: getCorsHeaders() });
+  try {
+    const result = streamText({
+      model: DEFAULT_MODEL,
+      system: buildCampaignSystemPrompt(),
+      prompt: buildCampaignUserPrompt(validated),
+    });
+    return result.toTextStreamResponse({ headers: getCorsHeaders() });
+  } catch {
+    return NextResponse.json(
+      { status: "error", error: "Failed to generate campaign" },
+      { status: 500, headers: getCorsHeaders() },
+    );
+  }
 }

Per coding guidelines, lib/**/*.ts domain functions require proper error handling.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const json = await request.json();
const validated = validateLaunchBody(json);
if (validated instanceof NextResponse) {
return validated;
}
const result = streamText({
model: DEFAULT_MODEL,
system: buildCampaignSystemPrompt(),
prompt: buildCampaignUserPrompt(validated),
});
return result.toTextStreamResponse({ headers: getCorsHeaders() });
}
let json: unknown;
try {
json = await request.json();
} catch {
return NextResponse.json(
{ status: "error", error: "Invalid JSON body" },
{ status: 400, headers: getCorsHeaders() },
);
}
const validated = validateLaunchBody(json);
if (validated instanceof NextResponse) {
return validated;
}
try {
const result = streamText({
model: DEFAULT_MODEL,
system: buildCampaignSystemPrompt(),
prompt: buildCampaignUserPrompt(validated),
});
return result.toTextStreamResponse({ headers: getCorsHeaders() });
} catch {
return NextResponse.json(
{ status: "error", error: "Failed to generate campaign" },
{ status: 500, headers: getCorsHeaders() },
);
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/launch/generateCampaignHandler.ts` around lines 22 - 35, Wrap the call to
request.json() in a try-catch and return a controlled JSON error response with
getCorsHeaders() if parsing fails, and likewise wrap the streamText(...)
invocation (the call that uses DEFAULT_MODEL, buildCampaignSystemPrompt(),
buildCampaignUserPrompt(validated)) in its own try-catch to catch generation
errors and return a consistent NextResponse.json error with CORS headers; keep
the existing validateLaunchBody flow (checking for NextResponse) and only call
streamText after validated succeeds, and ensure any caught error messages are
included in the JSON error body so handlers like
getAccountHandler/createCatalogSongsHandler style are followed.

Copy link
Copy Markdown
Collaborator Author

@recoup-coding-agent recoup-coding-agent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review: Recoup Content Agent Slack Bot

Summary

Adds a new Recoup Content Agent Slack bot that lets users trigger content generation via @RecoupContentAgent <artist_account_id> [template] [batch=N] [lipsync]. Also includes a new /api/launch streaming campaign endpoint that appears unrelated to the Slack bot feature.

Follows the existing coding-agent pattern: Chat SDK with Slack adapter, Redis-backed thread state, Trigger.dev polling, and a callback endpoint for async results.


CLEAN Code Assessment

SRP ✅

Core handlers are nicely separated: handleContentAgentMention.ts handles incoming mentions, handleContentAgentCallback.ts handles HTTP callbacks, validateContentAgentEnv.ts validates config. Each file has a clear single responsibility.

OCP ✅

Registration via registerHandlers.ts follows the open-for-extension pattern from the coding-agent. New handlers can be added without modifying existing routes.

DRY ⚠️

See naming confusion issue below.

YAGNI ⚠️

  • The /api/launch campaign endpoint has no connection to the content-agent Slack bot. It should be a separate PR.
  • Many existing files received boilerplate JSDoc additions with no content (e.g., @param body with no description). These add noise without value.

Issues Found

🔴 Blocking

1. Module-level bot singleton crashes on import when env vars are absent

lib/content-agent/bot.ts exports:

export const contentAgentBot = createContentAgentBot().registerSingleton();

createContentAgentBot() calls validateContentAgentEnv() which throws if SLACK_CONTENT_BOT_TOKEN, SLACK_CONTENT_SIGNING_SECRET, CONTENT_AGENT_CALLBACK_SECRET, or REDIS_URL are missing. Since this runs at import time, any file that imports from bot.ts will crash the entire Next.js build/start in environments where these vars aren't configured.

Recommend wrapping in a lazy getter:

let _contentAgentBot: ContentAgentBot | null = null;
export function getContentAgentBot() {
  if (!_contentAgentBot) _contentAgentBot = createContentAgentBot().registerSingleton();
  return _contentAgentBot;
}

🟡 Suggestions

2. Naming collision between two handleContentAgentCallback.ts files

  • lib/content-agent/handleContentAgentCallback.ts — HTTP callback handler (84 lines)
  • lib/content-agent/handlers/handleContentAgentCallback.ts — bot subscribed-message registration (17 lines)

Two files with identical names doing completely different things in sibling directories. The handler in handlers/ should be renamed to registerOnSubscribedMessage.ts (consistent with registerOnNewMention.ts next to it).

3. parseMentionArgs — use batch= prefix check consistently

} else if (!token.startsWith("batch") && token !== "lipsync") {

This guard uses !token.startsWith("batch") but the intent is to skip batch=N tokens. Use !token.startsWith("batch=") to be explicit and avoid accidentally filtering tokens like batchSize if future options are added.

4. /api/launch endpoint belongs in a separate PR

The streaming campaign generator has no runtime coupling to the content-agent Slack bot. Bundling it here makes the PR harder to review and harder to revert independently.


🔵 Nits

5. Boilerplate JSDoc on existing simple functions adds noise

Many existing files received @param JSDoc tags with no actual description body. Empty JSDoc is worse than no JSDoc.


Security

✅ Callback authentication uses a shared secret (x-callback-secret) — validated before processing.
✅ Zod validation on callback and launch bodies.
✅ No hardcoded secrets.
⚠️ The secret !== expectedSecret comparison is not timing-safe. Consider crypto.timingSafeEqual to prevent timing attacks (low-risk for internal callback, but worth noting).


Verdict: request-changes

Core feature is well-structured. Two changes needed before merge:

  1. Fix module-level env validation crash (blocking)
  2. Rename handlers/handleContentAgentCallback.tshandlers/registerOnSubscribedMessage.ts (maintainability)

Copy link
Copy Markdown
Collaborator Author

@recoup-coding-agent recoup-coding-agent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

Scaffolds a new Recoup Content Agent Slack bot following the existing coding-agent pattern. Handles @RecoupContentAgent <artist_account_id> [template] [batch=N] [lipsync] Slack mentions, triggers the create-content pipeline, polls via Trigger.dev (companion: recoupable/tasks#109), and posts video URLs back to the thread via Redis-backed Chat SDK state.

Also bundles in a new /api/launch campaign-generation endpoint (unrelated — see below).

CLEAN Code Assessment

  • SRP: Generally good. The handler chain (mention → trigger → poll → callback) is clean. One concern below.
  • OCP: Well-structured — new bot follows existing adapter pattern without modifying coding-agent code.
  • DRY: Shares resolveArtistSlug, getArtistContentReadiness, contentTemplates, Supabase helpers — good reuse.
  • YAGNI: A few instances of empty JSDoc noise (see nits).

Issues Found

blocking

Confusing duplicate filename (lib/content-agent/handlers/handleContentAgentCallback.ts):

This file exports registerOnSubscribedMessage, but its filename is handleContentAgentCallback.ts — identical to the sibling file at lib/content-agent/handleContentAgentCallback.ts that handles the actual Trigger.dev callback. This is a real confusion risk when navigating the codebase. Rename lib/content-agent/handlers/handleContentAgentCallback.ts to registerSubscribedMessage.ts (or handleSubscribedMessage.ts) to match its export.

suggestion

/api/launch bundled in this PR (app/api/launch/route.ts + lib/launch/):

This is an unrelated campaign-generation feature mixed into the content-agent PR. Unrelated features in the same PR make review harder and rollback riskier. Please split it into its own PR.

Dual-path githubRepo resolution (lib/content-agent/handlers/handleContentAgentMention.ts lines 70–87):

The try/catch fallback from getArtistContentReadiness to selectAccountSnapshots is implicit control flow via exception. If getArtistContentReadiness fails for infrastructure reasons (network, DB), it silently falls back to snapshots. Consider checking only one authoritative source, or separating the two paths with an explicit if/else rather than exception-driven branching.

ContentAgentThreadState has required fields (lib/content-agent/types.ts):

artistAccountId, template, lipsync, batch, runIds are all required in the interface, but setState is only called with a full object after triggerPollContentRun. The status: "running" is set atomically with all other fields — this is fine as written, but the type could use Partial<> or a discriminated union to represent intermediate states more precisely.

nit

  • batch max is hardcoded as 30 in parseMentionArgs — use a named constant MAX_CONTENT_BATCH_SIZE = 30.
  • Many empty @param JSDoc blocks were added to existing functions throughout the diff (e.g. @param url, @param body, @param request with no description). These add noise with no value — either add meaningful descriptions or omit them.
  • getThread in lib/content-agent/getThread.ts manually parses the thread ID format (adapterName:channelId). This is a tight coupling to Chat SDK internals. A comment explaining the expected format would help future maintainers.

Security

  • Callback protected by x-callback-secret shared secret — correct pattern.
  • Env vars validated at startup via validateContentAgentEnv() — good.
  • No hardcoded secrets.
  • No SQL injection risk (Supabase client used consistently).

Verdict

request-changes — two issues need resolution before merge:

  1. Rename lib/content-agent/handlers/handleContentAgentCallback.ts to avoid collision with lib/content-agent/handleContentAgentCallback.ts.
  2. Split the /api/launch route into a separate PR.

@recoup-coding-agent
Copy link
Copy Markdown
Collaborator Author

Code Review — Recoup Content Agent Slack Bot

Summary

Adds a Slack bot (@RecoupContentAgent) that lets users trigger the existing create-content pipeline via Slack mention. Also adds POST /api/launch for streaming AI-generated release campaigns. The core feature logic is well-structured and follows project conventions.

The blocking issue is not the feature code — it's the 90+ unrelated JSDoc-only changes bundled into this PR.


CLEAN Code Assessment

Principle Rating Notes
SRP New files each export one function
OCP Extends the coding-agent pattern without modifying it
DRY Reuses existing triggerCreateContent, auth helpers, Supabase pattern
YAGNI 90 files worth of trivial JSDoc @param annotations added to unrelated modules

Issues Found

🔴 Blocking

1. ~90 unrelated JSDoc-only changes must be removed

Out of 106 changed files, ~90 are existing files that received only trivial JSDoc additions like:

/**
 * @param request
 */
export async function GET(request: NextRequest) {

or even empty stubs:

/**
 *
 */
export async function OPTIONS() {

These touch completely unrelated modules: lib/admins/, lib/catalog/, lib/evals/, lib/coding-agent/, lib/credits/, lib/flamingo/, lib/github/, lib/mcp/, test files, etc. They have nothing to do with the content agent feature and:

  • Violate YAGNI — undocumented functions weren't blocking anything
  • Make the PR extremely hard to review (real changes buried in noise)
  • Introduce empty/meaningless JSDoc blocks that degrade rather than improve readability
  • This PR should contain only the minimum set of files required

Action required: Remove all JSDoc-only modifications to existing files. Keep only the new lib/content-agent/, lib/launch/, and app/api/content-agent|launch/ files, plus any genuinely required modifications to existing files.


🟡 Suggestion

2. Naming collision between two handleContentAgentCallback files

The PR creates two files with identical names in adjacent directories:

  • lib/content-agent/handleContentAgentCallback.ts — HTTP handler (84 lines)
  • lib/content-agent/handlers/handleContentAgentCallback.ts — bot onSubscribedMessage registrar (17 lines)

This is confusing and makes IDE navigation unreliable. Rename one — e.g. the HTTP handler → lib/content-agent/processContentAgentCallback.ts.

3. Use timing-safe secret comparison in callback handler

In lib/content-agent/handleContentAgentCallback.ts:

// Current — vulnerable to timing attacks
if (!secret || secret !== expectedSecret) {

// Better
import { timingSafeEqual } from "crypto";
const secretBuf = Buffer.from(secret);
const expectedBuf = Buffer.from(expectedSecret);
if (secretBuf.length !== expectedBuf.length || !timingSafeEqual(secretBuf, expectedBuf)) {

⚪ Nit

4. createContentAgentBot() validates env at module load time

validateContentAgentEnv() is invoked inside the factory which runs at module import (export const contentAgentBot = createContentAgentBot()). This throws at import time if env vars are missing — relevant for test isolation.


Security

  • Callback endpoint protected by shared secret ✅ (see timing-safe suggestion)
  • No hardcoded tokens ✅
  • No SQL injection risk — Supabase client throughout ✅
  • Auth check on generateCampaignHandler via validateAuthContext
  • Slack webhook challenge handshake via handleUrlVerification

Verdict: 🔴 Request Changes

The feature code itself is clean and follows project conventions. Strip the ~90 unrelated JSDoc-only file modifications, address the naming collision, and this is ready to merge.

@recoup-coding-agent
Copy link
Copy Markdown
Collaborator Author

CI Status Update — PR #341

Checking CI status as part of this review:

Check Status Result
test completed ✅ success
format completed 🔴 failure
Vercel deployment completed 🔴 failure

Format Check Failure (Blocking)

The format check is failing. This is likely caused by the JSDoc additions across ~90 files not conforming to Prettier formatting rules (consistent with the earlier blocking issue about unrelated JSDoc noise).

The author must run pnpm format (and optionally pnpm lint) locally and commit the fixes before this PR can merge.

Vercel Deployment Failure (Blocking)

The preview deployment also failed. This may be related to the format check or missing env vars. Check the Vercel logs:

npx vercel inspect dpl_WxpJ4W9q3ZCDvDEPsFtg8dMPDCWP --logs

New required env vars to add in Vercel:

  • SLACK_CONTENT_BOT_TOKEN
  • SLACK_CONTENT_SIGNING_SECRET
  • CONTENT_AGENT_CALLBACK_SECRET

Updated verdict: 🔴 Request Changes — in addition to stripping the JSDoc noise (previous comment), the format check failure and Vercel deployment failure must also be resolved before merge.

@recoup-coding-agent
Copy link
Copy Markdown
Collaborator Author

Superseded by #342 — clean PR with only 17 new feature files (no JSDoc noise). All review feedback addressed:

  • Removed ~90 unrelated JSDoc changes
  • Renamed handleContentAgentCallback.tsregisterOnSubscribedMessage.ts (naming collision fix)
  • Added crypto.timingSafeEqual for callback secret
  • Fixed all lint errors

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant